feat(weixin): add image sending support via CDN upload by Mr-Maidong · Pull Request #3781 · QwenLM/qwen-code

Mr-Maidong · 2026-05-01T16:15:46Z

Summary

What changed: 为微信渠道（WeChat channel）添加图片发送功能，通过 CDN
四步上传流程实现。
Why it changed: 原微信渠道仅支持文本消息，本次新增图片发送能力。
Reviewer focus: CDN 上传流程、图片标记解析、错误处理。

Changes

核心功能

send.ts: 新增 sendImage，实现四步 CDN
上传流程（读取→getuploadurl→加密+CDN上传→sendmessage）
api.ts: 新增 getUploadUrl 和 uploadToCdn
media.ts: 新增 encryptAesEcb、computeMd5，导出 parseAesKey
WeixinAdapter.ts: 自动注入图片能力说明、解析 [IMAGE: ...] 标记、发送失败回退

测试

send.test.ts: sendImage 单元测试（正常流程 + 错误传播）
media.test.ts: 加密和 MD5测试

Validation

单元测试: 29/29通过 ✅
覆盖: 加密往返、空输入、CDN 上传四步、错误传播路径

wenshao

⚠️ CI failing: CodeQL

This PR adds image sending to the WeChat channel via CDN upload. The review found 7 Critical issues — most notably an arbitrary file read vulnerability via AI-controlled [IMAGE: ...] paths with zero validation, an aes_key encoding mismatch that will cause sent images to be undecryptable on the receiving end, and unhandled rejections in error fallback paths. These must be fixed before merging.

Additional findings without inline comments:

[Suggestion] downloadAndDecrypt in media.ts also uses bare fetch() with no timeout — same issue as uploadToCdn. A stalled CDN download hangs the inbound message handler indefinitely. Add AbortController with timeout.
[Suggestion] Multiple message_state: FINISH per AI response — text + N images = N+1 sendMessage calls each declaring FINISH. The iLink protocol likely expects one FINISH per bot turn. Consider combining items in a single call, or using GENERATING for intermediate messages.
[Suggestion] uploadToCdn accepts http:// URLs — startsWith('http') matches cleartext URLs, sending encrypted data over unencrypted transport. Should use startsWith('https://').

wenshao · 2026-05-02T08:14:44Z

+  const { to, imagePath, baseUrl, token, contextToken } = params;
+
+  // Step 1: read file, compute metadata + generate random identifiers
+  const fileBuffer = readFileSync(imagePath);


[Critical] Arbitrary file read via AI-controlled path — no validation

readFileSync(imagePath) reads whatever path the AI generates in [IMAGE: ...] markers. There is zero path validation, sanitization, or allowlisting. A prompt injection in a WeChat user message can trick the AI into generating [IMAGE: /etc/shadow] or [IMAGE: ~/.ssh/id_rsa], causing the bot to read arbitrary files, encrypt them, and upload them to the CDN for exfiltration.

Additionally, there is no file size limit — [IMAGE: /dev/urandom] or a multi-GB file causes OOM/hang. No file type check either — any file is uploaded as media_type: 1 (image).

Suggested change

const fileBuffer = readFileSync(imagePath);

import { resolve, extname } from 'node:path';

import { statSync } from 'node:fs';

const ALLOWED_DIRS = ['/tmp/', process.env.IMAGE_OUTPUT_DIR].filter(Boolean) as string[];

const ALLOWED_EXTS = new Set(['.png', '.jpg', '.jpeg', '.gif', '.webp']);

const MAX_IMAGE_SIZE = 20 * 1024 * 1024; // 20 MB

function validateImagePath(imagePath: string): string {

const resolved = resolve(imagePath);

if (!ALLOWED_DIRS.some((d) => resolved.startsWith(d))) {

throw new Error(`Image path not in allowed directories: ${resolved}`);

}

if (!ALLOWED_EXTS.has(extname(resolved).toLowerCase())) {

throw new Error(`Image extension not allowed: ${extname(resolved)}`);

}

const st = statSync(resolved);

if (!st.isFile()) throw new Error('Not a regular file');

if (st.size > MAX_IMAGE_SIZE) throw new Error(`File too large: ${st.size} bytes`);

return resolved;

}

Call validateImagePath(imagePath) before readFileSync.

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

+  );
+
+  // Step 3: encrypt and upload to CDN
+  const encrypted = encryptAesEcb(fileBuffer, aesKeyBytes);


[Critical] aes_key encoding mismatch — sent images will be undecryptable

Buffer.from(aesKeyHex, 'ascii').toString('base64') produces base64 of a 32-char hex ASCII string. Decoding this base64 yields 44 bytes (32 ASCII chars + PKCS7 padding). But parseAesKey in media.ts only accepts decoded lengths of 16 (raw key) or 32 (hex string). 44 matches neither branch — the receiving side will throw "Invalid aes_key" and cannot decrypt the image.

This is the hardest bug to diagnose: uploads succeed, messages send without error, but the recipient sees a broken image with no error on the sender side.

Verify against the WeChat protocol which format is required. If base64(raw 16 bytes):

Suggested change

const encrypted = encryptAesEcb(fileBuffer, aesKeyBytes);

const aesKeyBase64 = aesKeyBytes.toString('base64');

If base64(hex string) is correct, then parseAesKey needs a branch for 44-byte decoded length.

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

+
+  const resp = await fetch(url, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/octet-stream' },


[Critical] CDN upload has no timeout — can hang indefinitely

uploadToCdn uses bare fetch() with no AbortController, unlike the post() helper which has a 40-second timeout. A stalled CDN server will hang this promise forever, blocking the sequential image-sending loop and the entire message pipeline for that user.

Suggested change

headers: { 'Content-Type': 'application/octet-stream' },

export async function uploadToCdn(

urlOrParam: string,

filekey: string,

encryptedData: Buffer,

): Promise<string> {

const controller = new AbortController();

const timeout = setTimeout(() => controller.abort(), 40000);

try {

const url = urlOrParam.startsWith('http')

? urlOrParam

: `https://novac2c.cdn.weixin.qq.com/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`;

const resp = await fetch(url, {

method: 'POST',

headers: { 'Content-Type': 'application/octet-stream' },

body: encryptedData,

signal: controller.signal,

});

if (!resp.ok) {

throw new Error(`CDN upload failed: HTTP ${resp.status}`);

}

const encryptParam = resp.headers.get('x-encrypted-param');

if (!encryptParam) {

throw new Error(

'CDN upload succeeded but missing x-encrypted-param header',

);

}

return encryptParam;

} finally {

clearTimeout(timeout);

}

}

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

+          process.stderr.write(
+            `[Weixin:${this.name}] Failed to send image ${imagePath}: ${errMsg}\n`,
+          );
+          await sendText({


[Critical] Fallback sendText not wrapped in try/catch — unhandled rejection

When sendImage fails, the catch block calls await sendText(...) to notify the user, but this fallback call is itself unguarded. If sendText also throws (e.g., expired auth token), the unhandled promise rejection can crash the process under Node.js's default --unhandled-rejections=throw.

Also, errMsg is sent directly to the user, leaking internal file paths (e.g., ENOENT: no such file or directory, open '/etc/shadow').

Suggested change

await sendText({

} catch (err) {

const errMsg = err instanceof Error ? err.message : String(err);

process.stderr.write(

`[Weixin:${this.name}] Failed to send image ${imagePath}: ${errMsg}\n`,

);

try {

await sendText({

to: chatId,

text: '图片发送失败，请稍后重试',

baseUrl: this.baseUrl,

token: this.token,

contextToken,

});

} catch (fallbackErr) {

process.stderr.write(

`[Weixin:${this.name}] Fallback text also failed: ${fallbackErr instanceof Error ? fallbackErr.message : String(fallbackErr)}\n`,

);

}

}

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z


+    // Always remind the AI about image-sending capability on every message
+    const IMAGE_INSTRUCTION =
+      '[WeChat Channel] 你可以通过微信发送图片。在回复中使用 [IMAGE: 文件绝对路径] 发送图片，例如 [IMAGE: /tmp/cat.png]。标记会被自动移除。';


[Suggestion] Per-message IMAGE_INSTRUCTION is redundant and increases attack surface

IMAGE_INSTRUCTION is prepended to every inbound message, but config.instructions already set in connect() tells the AI about image capability. This doubles token cost (~80 Chinese chars per message) and explicitly tells the AI the [IMAGE: ...] syntax on every turn, making prompt injection easier for an attacker.

Consider removing the per-message injection and relying solely on config.instructions. If a reminder is needed, keep it in the system prompt rather than the user message.

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

  }

-  async sendMessage(chatId: string, text: string): Promise<void> {
+  async sendMessage(


[Suggestion] imagePaths parameter is dead code

The base class ChannelBase.sendMessage(chatId, text) only passes 2 arguments. No caller ever provides imagePaths. This parameter gives the misleading impression that the ACP pipeline passes image paths, but it never does. Consider removing it until the pipeline actually supports it, or add a comment marking it as a future integration point.

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

+ *  If it's just a param, construct the URL. */
+export async function uploadToCdn(
+  urlOrParam: string,
+  filekey: string,


[Suggestion] SSRF — no URL host validation

uploadToCdn uses urlOrParam.startsWith('http') which accepts http:// URLs (sending encrypted data over cleartext) and does not validate the host. A compromised API response could direct the POST to an internal service (e.g., cloud metadata endpoint http://169.254.169.254/).

Suggested change

filekey: string,

const WECHAT_CDN_HOST = 'novac2c.cdn.weixin.qq.com';

let url: string;

if (urlOrParam.startsWith('https://')) {

const parsed = new URL(urlOrParam);

if (parsed.hostname !== WECHAT_CDN_HOST) {

throw new Error(`CDN upload URL has unexpected host: ${parsed.hostname}`);

}

url = urlOrParam;

} else {

url = `https://${WECHAT_CDN_HOST}/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`;

}

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

-      contextToken,
+
+    // Parse [IMAGE: /path/to/file.png] markers from text
+    const imageRegex = /\[IMAGE:\s*([^\]]+)\]/gi;


[Suggestion] [IMAGE:] regex matches inside code blocks and produces empty paths

Two issues with the regex \[IMAGE:\s*([^\]]+)\]:

If the AI explains the syntax inside a code block (`[IMAGE: /tmp/example.png]`), the marker is extracted and the code block text is corrupted.

[IMAGE: ] captures a space, which .trim() converts to '', then readFileSync('') throws a confusing error.

For (2), filter empty paths after trimming:

Suggested change

const imageRegex = /\[IMAGE:\s*([^\]]+)\]/gi;

let cleanedText = text.replace(imageRegex, (_, path: string) => {

const trimmed = path.trim();

if (trimmed) parsedImages.push(trimmed);

return '';

});

For (1), consider stripping code blocks before running the regex, or extracting the parsing into a testable pure function.

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

+    body,
+    token,
+  );
+


[Suggestion] getUploadUrl doesn't verify ret === 0

The function checks for upload_full_url/upload_param presence but never validates resp.ret === 0 first. This is inconsistent with getUpdates in monitor.ts which explicitly checks resp.ret. An error response with a non-zero ret but a non-empty upload_full_url would be used without validation.

Suggested change

if (resp.ret !== undefined && resp.ret !== 0) {

throw new Error(

`getuploadurl failed: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`,

);

}

Place this check before the upload_full_url / upload_param checks.

— pai/glm-5 via Qwen Code /review

wenshao · 2026-05-02T08:14:44Z

+  computeMd5: vi.fn(() => 'd41d8cd98f00b204e9800998ecf8427e'),
+}));
+
+const { sendImage } = await import('./send.js');


[Suggestion] Test mock hides encryption behavior — ciphertext size mismatch undetectable

encryptAesEcb is mocked as (data: Buffer) => data (identity, no PKCS7 padding). The test then asserts uploadToCdn receives fakeImageData (raw 16 bytes). In production, encryptAesEcb adds PKCS7 padding, producing 32 bytes. This means:

The test validates the wrong data (raw vs encrypted)

If the actual encrypted size diverges from the encryptedSize formula, the test won't catch it

Consider using the real encryptAesEcb/computeMd5 implementations in the test, or at minimum make the mock return a transformed buffer so the test validates the correct data flow.

Additionally, these test coverage gaps should be addressed:

readFileSync throwing ENOENT (file not found)

Step 4 sendMessage failure after successful CDN upload

getUploadUrl and uploadToCdn have zero test coverage

WeixinAdapter image-parsing logic is untested

— pai/glm-5 via Qwen Code /review

…error handling Critical fixes from wenshao's review of feat/weixin-image-send: 1. File read vulnerability: add validateImagePath() in send.ts with directory allowlist, extension filter, magic-byte check, 20MB cap, and realpath resolution. Pass workspace cwd as allowed dir. 2. aes_key encoding: change from base64(hex-ascii) to base64(raw 16B) to match the protocol expectation (images use raw bytes, not hex). 3. uploadToCdn timeout: add AbortController + 40s timeout per retry attempt to prevent hanging on stalled CDN connections. 4. Unhandled rejection: wrap fallback sendText() in catch block with its own try/catch to prevent process crash on double failure. 5. Default instructions merge: append image capability guide when custom instructions lack [IMAGE:], instead of silently dropping it. 6. Dead code: remove unused imagePaths parameter from sendMessage(). 7. Regex hardening: strip code blocks before [IMAGE:] extraction, filter empty paths to prevent confusing readFileSync('') errors. 8. URL validation: reject http:// URLs and validate CDN hostname in uploadToCdn (SSRF prevention). Tests: replace identity mock with real encryptAesEcb/computeMd5 so padding mismatches are caught; fix partial node:crypto mock. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Mr-Maidong · 2026-05-03T16:20:20Z

@wenshao Thanks for the thorough review! All issues addressed:

Critical fixes:

✅ File read vulnerability — validateImagePath() with directory allowlist, extension filter, magic-byte check, 20MB cap, realpath resolution
✅ aes_key encoding — base64(raw 16B) per protocol (images use raw bytes, not hex-ascii)
✅ uploadToCdn timeout — AbortController + 40s timeout per retry attempt
✅ Unhandled rejection — fallback sendText wrapped in its own try/catch
✅ Default instructions merge — append image capability guide when custom instructions lack [IMAGE:]
✅ Dead code — removed unused imagePaths parameter
✅ Regex hardening — strip code blocks before [IMAGE:] extraction, filter empty paths
✅ URL validation — reject http:// URLs, validate CDN hostname in uploadToCdn

Other improvements:

Tests use real encryptAesEcb/computeMd5 instead of identity mock
downloadAndDecrypt timeout/retry handled via existing retryWithBackoff

PTAL when you get a chance.

wenshao

测试覆盖建议（无法映射到具体行）

connect() imageInstructions 注入逻辑（3 个分支）未测试 — WeixinAdapter.ts:46-72
sendMessage 图片发送失败回退路径未测试 — WeixinAdapter.ts:214-241
validateImagePath 7 个错误分支未测试 — send.ts:64-112
detectImageMime GIF/WebP/JPEG 分支未测试 — send.ts:37-59
getUploadUrl 错误响应分支未测试 — api.ts:238-264
uploadToCdn URL 构建和 CDN 错误分支未测试 — api.ts:267-316

wenshao · 2026-05-03T22:04:16Z

+export function detectImageMime(data: Buffer): string {
+  if (
+    data[0] === 0x89 &&
+    data[1] === 0x50 &&


[Critical] detectImageMime 无法检测 JPEG — 安全边界失效

函数只检查 PNG/GIF/WebP 魔数，无 JPEG (FF D8 FF) 检测。任何文件改名为 .jpg 都通过 MIME 验证，使得 .jpg/.jpeg 文件的魔数检查完全失效。

Suggested change

data[1] === 0x50 &&

if (data[0] === 0xff && data[1] === 0xd8 && data[2] === 0xff) {

return 'image/jpeg';

}

throw new Error('Unrecognized image format');

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+    rawfilemd5,
+    filesize: encryptedSize,
+    no_need_thumb: true,
+    aeskey: aeskeyHex,


[Critical] getUploadUrl 重试永不生效 — isRetryableError 检查 errcode 但此端点使用 ret

GetUploadUrlResp 接口没有 errcode 字段，WeixinApiError 构造函数只传入 ret（第 3 参数）未传 errcode（第 4 参数）。isRetryableError 只检查 err.errcode（=== -14、=== -1、=== 45011），因此来自 getuploadurl 的所有 API 错误都直接传播，不重试。

Suggested change

aeskey: aeskeyHex,

// 在 GetUploadUrlResp 中添加 errcode?: number

// 映射 resp.errcode 并传入 WeixinApiError 构造函数

if (resp.ret !== undefined && resp.ret !== 0 || resp.errcode !== undefined && resp.errcode !== 0) {

throw new WeixinApiError(

`getuploadurl failed: ret=${resp.ret} errcode=${resp.errcode ?? '(none)'} errmsg=${resp.errmsg || '(none)'}`,

200,

resp.ret,

resp.errcode,

);

}

// 同时在 isRetryableError 中增加对 err.ret 的检查

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+  })();
+
+  const st = statSync(real);
+  if (!st.isFile()) {


[Critical] macOS tmpdir() 路径不匹配 — 所有临时目录图片被拒绝

realpathSync 将 /var 解析为 /private/var（跟随符号链接），但 os.tmpdir() 返回原始 /var/folders/.../T。real.startsWith(dir) 比较失败，macOS 上 /tmp/ 下所有图片被拒绝。测试 mock tmpdir 为 '/tmp' 掩盖了此问题。

Suggested change

if (!st.isFile()) {

const ALLOWED_DIRS = [

'/tmp/',

'/private/tmp/',

realpathSync(tmpdir()) + '/',

...workspaceDirs.map((d) => realpathSync(resolve(d)) + '/'),

];

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+    const textWithoutCode = text
+      .replace(/```[\s\S]*?```/g, '')
+      .replace(/`[^`]*`/g, '');
+


[Critical] 代码块内 [IMAGE:] 标记被静默剥离 — 数据丢失

正则替换 text.replace(imageRegex, '') 作用于包含代码块的原始文本。代码块内的标记被移除但不解析为图片，用户看到的内容被静默篡改。

Suggested change

// 仅替换实际解析为图片的标记，而非全局替换

let cleanedText = text;

for (const img of parsedImages) {

cleanedText = cleanedText.replace(/\[IMAGE:\s*[^\]]+\]/i, '');

}

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+  }
+
+  // Verify magic bytes match the extension
+  const head = readFileSync(real, { flag: 'r' });


[Critical] 文件双重读取 + TOCTOU 竞态 — 每次上传浪费 2x I/O

validateImagePath 调用 readFileSync 读取整个文件仅检查魔数，随后 sendImage 再次 readFileSync 读取完整文件。大文件（最大 20MB）存在 2x 内存分配和 I/O，且验证和上传之间存在 TOCTOU 竞态窗口。

Suggested change

const head = readFileSync(real, { flag: 'r' });

// validateImagePath 中仅读取魔数所需字节（16 字节）：

const fd = openSync(real, 'r');

const head = Buffer.alloc(16);

try {

readSync(fd, head, 0, 16, 0);

} finally {

closeSync(fd);

}

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+    }
+  });
 }



[Critical] sendMessage 只检查 ret，忽略 errcode — 消息可能静默丢失

成功检测条件 resp.ret !== undefined && resp.ret !== 0 不检查 errcode。若 API 返回 { errcode: 45011, errmsg: "hit frequency limit" } 且不含 ret 字段，函数静默返回，文本和图片消息在最后一步丢失而调用方无感知。

Suggested change

if ((resp.ret !== undefined && resp.ret !== 0) ||

(resp.errcode !== undefined && resp.errcode !== 0)) {

throw new WeixinApiError(

`sendMessage failed: ret=${resp.ret} errcode=${resp.errcode} errmsg=${resp.errmsg || '(none)'}`,

200,

resp.ret,

resp.errcode,

);

}

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+      '',
+      'CRITICAL: Only use real file paths. Do NOT write [IMAGE: ...] with:',
+      '- Example paths like /path/to/file or /tmp/cat.png',
+      '- Placeholder symbols like ...',


[Suggestion] connect() 重复调用会污染 config.instructions

this.config.instructions += '\n' + imageInstructions 在原地修改配置对象。通道重连时（崩溃恢复），imageInstructions 会被重复追加，使配置持续膨胀。

Suggested change

'- Placeholder symbols like ...',

// 使用局部变量，不修改 this.config

const instructions = this.config.instructions + '\n' + imageInstructions;

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+    token,
+    to,
+    filekey,
+    rawsize,


[Suggestion] 重复「Step 2」注释 — 误导代码流程

函数体内有两个 // Step 2: 注释。JSDoc 描述 4 步流程，行内注释有 5 步标注，两者不一致。

Suggested change

rawsize,

// Step 3: get upload URL and CDN credentials

const uploadParam = await getUploadUrl(

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+    media_type: 1,
+    to_user_id: toUserId,
+    rawsize,
+    rawfilemd5,


[Suggestion] 错误消息中包含字面量 ${undefined}

`getuploadurl failed: ret=${resp.ret} errcode=${undefined} errmsg=...` 产生 errcode=undefined，误导排查人员。

Suggested change

rawfilemd5,

`getuploadurl failed: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-03T22:04:16Z

+    if (parsedImages.length) {
+      const workspaceDirs = [this.config.cwd];
+
+      for (const imagePath of parsedImages) {


[Suggestion] API errmsg 敏感信息泄露到 stderr

process.stderr.write(\[Weixin:${this.name}] Failed to send image ${imagePath}: ${errMsg}\n`)将原始 API 错误消息（含errmsg`）写入 stderr。若 WeChat API 在错误响应中返回 token 或用户标识等敏感数据，会被记录到日志聚合系统。

Suggested change

for (const imagePath of parsedImages) {

`[Weixin:${this.name}] Failed to send image (status=${err.status} ret=${err.ret})`

— deepseek-v4-pro via Qwen Code /review

Critical fixes: 1. detectImageMime: add JPEG magic bytes (0xFF 0xD8 0xFF), throw on unrecognized format instead of defaulting to image/jpeg 2. getUploadUrl retry: pass errcode to WeixinApiError, add ret field check in isRetryableError so actual API errors trigger retries 3. ALLOWED_DIRS: add realpathSync('/tmp/') and realpathSync(tmpdir()) to handle macOS symlink resolution (/tmp → /private/tmp) 4. [IMAGE:] stripping: only replace markers that were actually parsed, preserving [IMAGE:] inside code blocks in displayed text 5. TOCTOU fix: use openSync/readSync(16B) for magic-byte check instead of reading the entire file twice 6. sendMessage: check both ret and errcode fields for error detection Suggestions: 7. connect(): avoid mutating this.config.instructions on reconnect 8. Fix duplicate Step 2 comment numbering 9. Replace errcode=${undefined} with errcode=${resp.errcode ?? '(none)'} 10. stderr: log structured (status=, ret=) instead of raw errmsg Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Mr-Maidong · 2026-05-04T12:04:33Z

Hi @wenshao,

Second round of fixes pushed (17600d569). Here is what was addressed:

Critical fixes (6)

detectImageMime JPEG magic — Added 0xFF 0xD8 0xFF detection; default now throws Unrecognized image format instead of silently returning image/jpeg
getUploadUrl retry now works — Added errcode to GetUploadUrlResp interface, passed it to WeixinApiError constructor, and added ret field check in isRetryableError. Previously all errors hit the 4xx branch and were never retried
macOS tmpdir path mismatch — ALLOWED_DIRS now uses realpathSync('/tmp/') + '/' and realpathSync(tmpdir()) + '/' to handle macOS symlink resolution (/tmp to /private/tmp)
[IMAGE:] in code blocks preserved — Only markers whose paths were actually parsed outside code blocks are removed from the displayed text. Uses escapeRegex for exact matching
TOCTOU / double-read fix — validateImagePath now uses openSync/readSync to read only 16 bytes for magic-byte check, avoiding the full-file double-read and the race window
sendMessage checks errcode — Error condition is now (resp.ret !== 0) || (resp.errcode !== 0) so responses with errcode: 45011 (rate limit) are no longer silently ignored

Suggestions (4)

connect() no longer mutates this.config.instructions on reconnect — uses = assignment instead of +=
Fixed duplicate Step 2 comment numbering in sendImage()
Fixed errcode=${undefined} to errcode=${resp.errcode ?? '(none)'} in error messages
stderr no longer leaks raw errmsg — Logs structured (status=, ret=) only

All tests pass (29/29), typecheck clean, lint clean. Please review when you have a chance!

wenshao · 2026-05-04T14:23:44Z

+            workspaceDirs,
+          });
+        } catch (err) {
+          const status = err instanceof WeixinApiError ? err.status : 0;


[Suggestion] 错误日志丢失诊断信息 — 非 WeixinApiError 时完全不可用

错误日志从 Failed to send image ${imagePath}: ${errMsg} 改为 Failed to send image (status=${status} ret=${ret})。对于非 WeixinApiError 异常（文件 I/O 错误、路径校验失败等），日志输出无意义的 status=0 ret=undefined，无法区分文件不存在、权限拒绝还是网络超时。同时 errcode 字段虽已传入 WeixinApiError 但日志未提取。

Suggested change

const status = err instanceof WeixinApiError ? err.status : 0;

const status = err instanceof WeixinApiError ? err.status : 0;

const ret = err instanceof WeixinApiError ? err.ret : undefined;

const errcode = err instanceof WeixinApiError ? err.errcode : undefined;

const msg = err instanceof Error ? err.message : String(err);

process.stderr.write(

`[Weixin:${this.name}] Failed to send image (status=${status} ret=${ret} errcode=${errcode}): ${msg}\n`,

);

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-04T14:23:45Z

+      token,
+    );
+
+    // Check API-level error first


[Suggestion] getUploadUrl 缺少 errcode 错误检查 — 与 sendMessage 不一致

同一 PR 中 sendMessage（第 210 行）同时检查 ret !== 0 和 errcode !== 0，但 getUploadUrl 只检查 ret !== 0。如果微信 API 返回 {ret: 0, errcode: -1}，错误会被静默吞掉，落入「no URL」分支。

Suggested change

// Check API-level error first

if (

(resp.ret !== undefined && resp.ret !== 0) ||

(resp.errcode !== undefined && resp.errcode !== 0)

) {

— deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-04T14:23:45Z

+    realpathSync('/tmp/') + '/',
+    tmpdir() + '/',
+    realpathSync(tmpdir()) + '/',
+    ...workspaceDirs.map((d) => resolve(d) + '/'),


[Suggestion] workspace 目录未使用 realpathSync 解析符号链接 — 与 /tmp 处理不一致

临时目录（/tmp/、tmpdir()）已使用 realpathSync 解析符号链接，但 workspace 目录仅使用 resolve()。当工作目录包含符号链接时，validateImagePath 中 real（通过 realpathSync(imagePath) 解析）可能不匹配未解析的 workspace 目录前缀，合法图片被拒绝。

Suggested change

...workspaceDirs.map((d) => resolve(d) + '/'),

...workspaceDirs.map((d) => realpathSync(resolve(d)) + '/'),

— deepseek-v4-pro via Qwen Code /review

…eout, path resolution - api.ts: add errcode check in getUploadUrl (align with sendMessage) - api.ts: pass ret/errcode from CDN error to WeixinApiError - send.ts: resolve workspace dirs with realpathSync - WeixinAdapter.ts: include errcode and err.message in error log - media.ts: add 40s timeout to downloadAndDecrypt fetch - send.test.ts: add 12 tests covering detectImageMime and validateImagePath error branches Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Mr-Maidong · 2026-05-05T14:58:20Z

Third round of fixes pushed (cbcc0f747). Here is what was addressed:

Fixes

api.ts: getUploadUrl errcode check — Added resp.errcode !== 0 check alongside resp.ret, aligning error detection with sendMessage. Also passes ret/errcode from CDN upload error to WeixinApiError
send.ts: workspace dir resolution — Added realpathSync() when building ALLOWED_DIRS from workspace dirs, ensuring consistent path comparison on macOS
media.ts: download timeout — Added 40s timeout to downloadAndDecrypt fetch call to prevent indefinite hangs on slow/failed downloads
WeixinAdapter.ts: structured error logging — Now includes errcode and err.message in image send error output for better debugging
send.test.ts: +12 new tests — Covers detectImageMime all format branches (PNG/GIF/WebP/JPEG/unrecognized) and validateImagePath error branches (not found, invalid ext, directory traversal, size limit)

All tests pass (41/41), typecheck clean, lint clean. Please review!

wenshao

LGTM. Verified locally:

41/41 unit tests pass (vitest)
tsc --build clean
eslint clean (0 warnings)
Merges cleanly with main

Implementation looks solid:

Path validation uses realpathSync + allowlist + magic-byte check, with TOCTOU mitigation
SSRF guard on CDN upload (HTTPS only, hostname pinned to novac2c.cdn.weixin.qq.com)
Retry policy correctly distinguishes transient vs terminal errors (errcode -14 not retried)
Fallback sendText is wrapped in its own try/catch to avoid unhandled rejections on double failure
Random AES key per upload via crypto.randomBytes

Three rounds of review feedback were addressed thoroughly.

wenshao · 2026-05-05T17:49:58Z

本地验证报告

在合并前对 9c3964c 做了完整的本地验证，记录如下供参考。

环境

工作目录：基于 main 分支 fetch pull/3781/head
仓库根目录运行 npx vitest、npx tsc --build、npx eslint

测试结果

项	命令	结果
单元测试	`npx vitest run` (in `packages/channels/weixin`)	✅ 41/41 passed (2 test files, 271ms)
类型检查	`npx tsc --build packages/channels/weixin`	✅ 0 errors
Lint	`npx eslint packages/channels/weixin --ext .ts`	✅ 0 warnings, 0 errors

测试明细：

media.test.ts: 11 tests — parseAesKey (4) / decryptAesEcb (2) / encryptAesEcb (3) / computeMd5 (2)
send.test.ts: 30 tests — markdownToPlainText (15) / detectImageMime (5) / validateImagePath (7) / sendImage (3)

PR 描述里写的是 29 个测试，那是首版数据。3 轮 review 后追加了 12 个用例（魔数分支、validateImagePath 错误分支等），覆盖比首版更全。

实现关键点验证

路径校验（send.ts:88-158）：realpathSync 解 symlink → 扩展名白名单 → 大小上限（20MB）→ openSync/readSync(16) 校验魔数（避免 TOCTOU 双读）→ 允许目录前缀匹配（/tmp/、/private/tmp/、os.tmpdir()、workspaceDirs）
SSRF 防护（api.ts:338-356）：full URL 必须 HTTPS 且 hostname == novac2c.cdn.weixin.qq.com
重试策略（api.ts:36-54）：5xx + 网络错误 + errcode -1/45011 重试；4xx + errcode -14（session 过期）不重试
回退健壮性（WeixinAdapter.ts:248-272）：图片发送失败回退发文本，fallback 自带 try/catch 防双重失败时的 unhandled rejection
AES key 编码（send.ts:236）：images 用 base64(raw 16 bytes)，与协议一致

CI

GitHub Actions 全绿（Lint / CodeQL / 9 个 Test 矩阵：macOS/Ubuntu/Windows × Node 20/22/24）。

LGTM, merging.

* feat(weixin): add image sending support via CDN upload * fix(weixin): address PR review — path validation, encoding, timeout, error handling Critical fixes from wenshao's review of feat/weixin-image-send: 1. File read vulnerability: add validateImagePath() in send.ts with directory allowlist, extension filter, magic-byte check, 20MB cap, and realpath resolution. Pass workspace cwd as allowed dir. 2. aes_key encoding: change from base64(hex-ascii) to base64(raw 16B) to match the protocol expectation (images use raw bytes, not hex). 3. uploadToCdn timeout: add AbortController + 40s timeout per retry attempt to prevent hanging on stalled CDN connections. 4. Unhandled rejection: wrap fallback sendText() in catch block with its own try/catch to prevent process crash on double failure. 5. Default instructions merge: append image capability guide when custom instructions lack [IMAGE:], instead of silently dropping it. 6. Dead code: remove unused imagePaths parameter from sendMessage(). 7. Regex hardening: strip code blocks before [IMAGE:] extraction, filter empty paths to prevent confusing readFileSync('') errors. 8. URL validation: reject http:// URLs and validate CDN hostname in uploadToCdn (SSRF prevention). Tests: replace identity mock with real encryptAesEcb/computeMd5 so padding mismatches are caught; fix partial node:crypto mock. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(weixin): address 2nd round PR review — 10 issues Critical fixes: 1. detectImageMime: add JPEG magic bytes (0xFF 0xD8 0xFF), throw on unrecognized format instead of defaulting to image/jpeg 2. getUploadUrl retry: pass errcode to WeixinApiError, add ret field check in isRetryableError so actual API errors trigger retries 3. ALLOWED_DIRS: add realpathSync('/tmp/') and realpathSync(tmpdir()) to handle macOS symlink resolution (/tmp → /private/tmp) 4. [IMAGE:] stripping: only replace markers that were actually parsed, preserving [IMAGE:] inside code blocks in displayed text 5. TOCTOU fix: use openSync/readSync(16B) for magic-byte check instead of reading the entire file twice 6. sendMessage: check both ret and errcode fields for error detection Suggestions: 7. connect(): avoid mutating this.config.instructions on reconnect 8. Fix duplicate Step 2 comment numbering 9. Replace errcode=${undefined} with errcode=${resp.errcode ?? '(none)'} 10. stderr: log structured (status=, ret=) instead of raw errmsg Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(weixin): 3rd round PR review — errcode checks, error logging, timeout, path resolution - api.ts: add errcode check in getUploadUrl (align with sendMessage) - api.ts: pass ret/errcode from CDN error to WeixinApiError - send.ts: resolve workspace dirs with realpathSync - WeixinAdapter.ts: include errcode and err.message in error log - media.ts: add 40s timeout to downloadAndDecrypt fetch - send.test.ts: add 12 tests covering detectImageMime and validateImagePath error branches Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Maidong <408097061@qq.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

feat(weixin): add image sending support via CDN upload

9149336

github-actions Bot mentioned this pull request May 2, 2026

📊 AI CLI 工具社区动态日报 2026-05-02 gsscsd/big_model_radar#280

Open

tanzhenxin added the type/feature-request New feature or enhancement request label May 2, 2026

github-actions Bot mentioned this pull request May 2, 2026

📊 AI CLI 工具社区动态日报 2026-05-02 zx0828/big_model_radar#5

Open

wenshao requested changes May 2, 2026

View reviewed changes

This was referenced May 2, 2026

📊 AI CLI 工具社区动态日报 2026-05-02 zx0828/big_model_radar#10

Open

📊 AI CLI 工具社区动态日报 2026-05-02 zx0828/big_model_radar#14

Open

📊 AI CLI 工具社区动态日报 2026-05-02 zx0828/big_model_radar#19

Open

Mr-Maidong and others added 3 commits May 2, 2026 21:56

Merge branch 'QwenLM:main' into feat/weixin-image-send

f85bbab

Merge branch 'QwenLM:main' into feat/weixin-image-send

dbf3da8

wenshao requested changes May 3, 2026

View reviewed changes

wenshao reviewed May 4, 2026

View reviewed changes

Merge branch 'QwenLM:main' into feat/weixin-image-send

9c3964c

wenshao approved these changes May 5, 2026

View reviewed changes

wenshao merged commit f4a9f7b into QwenLM:main May 5, 2026
13 checks passed

yiliang114 mentioned this pull request May 9, 2026

Weixin channel bot 发送图片后微信端显示灰色占位 #3993

Closed

xaelistic pushed a commit to xaelistic/qwen-code that referenced this pull request Jun 7, 2026

fix(docs): clarify global installation in readme (QwenLM#3781)

5e650c0

-  const fileBuffer = readFileSync(imagePath);
+import { resolve, extname } from 'node:path';
+import { statSync } from 'node:fs';
+const ALLOWED_DIRS = ['/tmp/', process.env.IMAGE_OUTPUT_DIR].filter(Boolean) as string[];
+const ALLOWED_EXTS = new Set(['.png', '.jpg', '.jpeg', '.gif', '.webp']);
+const MAX_IMAGE_SIZE = 20 * 1024 * 1024; // 20 MB
+function validateImagePath(imagePath: string): string {
+  const resolved = resolve(imagePath);
+  if (!ALLOWED_DIRS.some((d) => resolved.startsWith(d))) {
+    throw new Error(`Image path not in allowed directories: ${resolved}`);
+  }
+  if (!ALLOWED_EXTS.has(extname(resolved).toLowerCase())) {
+    throw new Error(`Image extension not allowed: ${extname(resolved)}`);
+  }
+  const st = statSync(resolved);
+  if (!st.isFile()) throw new Error('Not a regular file');
+  if (st.size > MAX_IMAGE_SIZE) throw new Error(`File too large: ${st.size} bytes`);
+  return resolved;
+}

	const encrypted = encryptAesEcb(fileBuffer, aesKeyBytes);
	const aesKeyBase64 = aesKeyBytes.toString('base64');

-    headers: { 'Content-Type': 'application/octet-stream' },
+export async function uploadToCdn(
+  urlOrParam: string,
+  filekey: string,
+  encryptedData: Buffer,
+): Promise<string> {
+  const controller = new AbortController();
+  const timeout = setTimeout(() => controller.abort(), 40000);
+  try {
+    const url = urlOrParam.startsWith('http')
+      ? urlOrParam
+      : `https://novac2c.cdn.weixin.qq.com/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`;
+    const resp = await fetch(url, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/octet-stream' },
+      body: encryptedData,
+      signal: controller.signal,
+    });
+    if (!resp.ok) {
+      throw new Error(`CDN upload failed: HTTP ${resp.status}`);
+    }
+    const encryptParam = resp.headers.get('x-encrypted-param');
+    if (!encryptParam) {
+      throw new Error(
+        'CDN upload succeeded but missing x-encrypted-param header',
+      );
+    }
+    return encryptParam;
+  } finally {
+    clearTimeout(timeout);
+  }
+}

-  filekey: string,
+  const WECHAT_CDN_HOST = 'novac2c.cdn.weixin.qq.com';
+  let url: string;
+  if (urlOrParam.startsWith('https://')) {
+    const parsed = new URL(urlOrParam);
+    if (parsed.hostname !== WECHAT_CDN_HOST) {
+      throw new Error(`CDN upload URL has unexpected host: ${parsed.hostname}`);
+    }
+    url = urlOrParam;
+  } else {
+    url = `https://${WECHAT_CDN_HOST}/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`;
+  }

-    const imageRegex = /\[IMAGE:\s*([^\]]+)\]/gi;
+    let cleanedText = text.replace(imageRegex, (_, path: string) => {
+      const trimmed = path.trim();
+      if (trimmed) parsedImages.push(trimmed);
+      return '';
+    });

+  if (resp.ret !== undefined && resp.ret !== 0) {
+    throw new Error(
+      `getuploadurl failed: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`,
+    );
+  }

-    data[1] === 0x50 &&
+  if (data[0] === 0xff && data[1] === 0xd8 && data[2] === 0xff) {
+    return 'image/jpeg';
+  }
+  throw new Error('Unrecognized image format');

-    aeskey: aeskeyHex,
+    // 在 GetUploadUrlResp 中添加 errcode?: number
+    // 映射 resp.errcode 并传入 WeixinApiError 构造函数
+    if (resp.ret !== undefined && resp.ret !== 0 || resp.errcode !== undefined && resp.errcode !== 0) {
+      throw new WeixinApiError(
+        `getuploadurl failed: ret=${resp.ret} errcode=${resp.errcode ?? '(none)'} errmsg=${resp.errmsg || '(none)'}`,
+,
+        resp.ret,
+        resp.errcode,
+      );
+    }
+    // 同时在 isRetryableError 中增加对 err.ret 的检查

+    // 仅替换实际解析为图片的标记，而非全局替换
+    let cleanedText = text;
+    for (const img of parsedImages) {
+      cleanedText = cleanedText.replace(/\[IMAGE:\s*[^\]]+\]/i, '');
+    }

-  const head = readFileSync(real, { flag: 'r' });
+// validateImagePath 中仅读取魔数所需字节（16 字节）：
+const fd = openSync(real, 'r');
+const head = Buffer.alloc(16);
+try {
+  readSync(fd, head, 0, 16, 0);
+} finally {
+  closeSync(fd);
+}

	'- Placeholder symbols like ...',
	// 使用局部变量，不修改 this.config
	const instructions = this.config.instructions + '\n' + imageInstructions;

	rawsize,
	// Step 3: get upload URL and CDN credentials
	const uploadParam = await getUploadUrl(

	rawfilemd5,
	`getuploadurl failed: ret=${resp.ret} errmsg=${resp.errmsg \|\| '(none)'}`

	for (const imagePath of parsedImages) {
	`[Weixin:${this.name}] Failed to send image (status=${err.status} ret=${err.ret})`

-          const status = err instanceof WeixinApiError ? err.status : 0;
+          const status = err instanceof WeixinApiError ? err.status : 0;
+          const ret = err instanceof WeixinApiError ? err.ret : undefined;
+          const errcode = err instanceof WeixinApiError ? err.errcode : undefined;
+          const msg = err instanceof Error ? err.message : String(err);
+          process.stderr.write(
+            `[Weixin:${this.name}] Failed to send image (status=${status} ret=${ret} errcode=${errcode}): ${msg}\n`,
+          );

-    // Check API-level error first
+    if (
+      (resp.ret !== undefined && resp.ret !== 0) ||
+      (resp.errcode !== undefined && resp.errcode !== 0)
+    ) {

	...workspaceDirs.map((d) => resolve(d) + '/'),
	...workspaceDirs.map((d) => realpathSync(resolve(d)) + '/'),

Uh oh!

Conversation

Mr-Maidong commented May 1, 2026

Summary

Changes

核心功能

测试

Validation

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mr-Maidong commented May 3, 2026

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mr-Maidong commented May 4, 2026

Critical fixes (6)

Suggestions (4)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mr-Maidong commented May 5, 2026

Fixes

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wenshao commented May 5, 2026

本地验证报告

环境

测试结果

实现关键点验证

CI